-
Notifications
You must be signed in to change notification settings - Fork 551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
parse the rekor bundle correctly #3461
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3461 +/- ##
==========================================
- Coverage 40.10% 40.06% -0.04%
==========================================
Files 155 155
Lines 10044 10046 +2
==========================================
- Hits 4028 4025 -3
- Misses 5530 5535 +5
Partials 486 486 ☔ View full report in Codecov by Sentry. |
bb4c296
to
432f9a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to add an e2e test for this? It looks like
Line 922 in 14795db
func TestAttachWithRekorBundle(t *testing.T) { |
cmd/cosign/cli/options/attach.go
Outdated
@@ -58,7 +58,7 @@ func (o *AttachSignatureOptions) AddFlags(cmd *cobra.Command) { | |||
"signing certificate and end with the root certificate. Included in the OCI Signature") | |||
cmd.Flags().StringVar(&o.TimeStampedSig, "tsr", "", | |||
"path to the Time Stamped Signature Response from RFC3161 compliant TSA") | |||
cmd.Flags().StringVar(&o.RekorBundle, "rekor-response", "", | |||
cmd.Flags().StringVar(&o.RekorResponse, "rekor-response", "", | |||
"path to the rekor bundle") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the description to note this can either be the output from cosign sign --bundle
or a Rekor response formatted as a RekorBundle struct?
if err != nil { | ||
return err | ||
} | ||
|
||
var localCosignPayload cosign.LocalSignedPayload | ||
err = json.Unmarshal(rekorBundleByte, &localCosignPayload) | ||
err = json.Unmarshal(rekorResponseByte, &localCosignPayload) | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We wouldn't want to return on the first error, since the input may be one of two structures. We'll want something like:
var localCosignPayload cosign.LocalSignedPayload
if err := json.Unmarshal(rekorResponseByte, &localCosignPayload); err == nil {
rekorBundle = localCosignPayload.Bundle
} else {
err := json.Unmarshal(rekorResponseByte, rekorBundle)
if err != nil {
return fmt.Errorf("unable to parse Rekor response to attach to image")
}
}
This also removes the need for the extra duplicateLocalCosignPayload
variable since you can use rekorBundle
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment still relevant. Line 110 would return immediately if there was an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viveksahu26 Just bumping
7b11555
to
610713e
Compare
@viveksahu26 please see the open comment, the function returns early if there is an error parsing the provided response without trying the other format |
610713e
to
d25cfba
Compare
hey @haydentherapper tried and updated the code. One more thing should I add test for different scenarios, like:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @viveksahu26, thanks for the updates. Sorry for pivoting on this PR, but I've been thinking a bit more about this feature.
The original purpose of the rekor-response
flag was to attach a rekor proof from when cosign sign
or cosign sign-blob
was used. It's fair to say that rekor-response
isn't the most accurate flag name, but this was intentional to avoid reusing "bundle".
With the change you've proposed here, we would add support for attaching a response directly from rekor. However this proposes the flag pointing to a RekorBundle
, which is an internal struct within Cosign. Thinking more, I don't think this is the right structure to provide via a flag, because no tool outputs this. A client would have to manually construct this struct from an actual rekor response, and this means having knowledge of how the struct is formed, which means reading the code. This would not be an ideal user experience.
What if we instead supported attaching an actual rekor response? The rekor-response
flag would point to a file containing the output from curl https://rekor.sigstore.dev/api/v1/log/entries?logIndex=60606559
, and Cosign would be responsible for crafting the correct struct to attach to the OCI manifest. How does this sound? This would be a much better user experience and not require manual crafting - a) upload to rekor via API or rekor-cli, b) fetch the response from rekor, c) upload to Cosign via cosign attach
.
I agree with the points you discussed, especially the output of ATM, we should replace this Actual curl https://rekor.sigstore.dev/api/v1/log/entries\?logIndex\=67934953 | jq
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 2931 0 2931 0 0 6572 0 --:--:-- --:--:-- --:--:-- 6586
{
"24296fb24b8ad77a8ada322cbba201d23b88acd2f68b29e358668e3aef36306ddb2c253ca0dc9ede": {
"body": "eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiI3YTIxODlmNzNlYTVkYmVlMmQ1NjgxMGU4NjBjZDgxNjdlYTFiMGYzMTdkZGRjMmU0YzE2NmU3ZDY4NzUzOGYwIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FUUNJQlZtMVc1YlNSOXBTYVFyRWRVL2dOeEZuaVQzMCs4RGp5SG9naERwWHM3b0FpQXMyby82c3l2bzRaTDd3YVUrMXBNeVIvR1dNWlZTaUdzRm5hSm04ZDZZZ0E9PSIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCUVZVSk1TVU1nUzBWWkxTMHRMUzBLVFVacmQwVjNXVWhMYjFwSmVtb3dRMEZSV1VsTGIxcEplbW93UkVGUlkwUlJaMEZGUldGSWNHUmhZVE13ZEVOaVMxbG5lalpyUlhsQmNqTXJORmh5TWdwb1pWUjNaM2hQVVVrM2QwcDRiVWhIWW5OU1dYcEJWbFJyUVN0RGIzVjZUblZrTUc5eGRuUTRXVXB0Tm5CQlFUZG5SbUZFUkZGU05EUlJQVDBLTFMwdExTMUZUa1FnVUZWQ1RFbERJRXRGV1MwdExTMHRDZz09In19fX0=",
"integratedTime": 1706680021,
"logID": "c0d23d6ad406973f9559f3ba2d1ca01f84147d8ffc5b8445c224f98b9591801d",
"logIndex": 67934953,
"verification": {
"inclusionProof": {
"checkpoint": "rekor.sigstore.dev - 2605736670972794746\n63781918\nHfsAZNXTujUoN6lWTZS92XQqF2k0Hdrc6YZ4Onnc5kI=\nTimestamp: 1706681910431084977\n\n— rekor.sigstore.dev wNI9ajBEAiBAw+qvNLfEP0qj18Vywbp4iuuj4LKcvb9PxlYnVCFt5AIgS/+5j2fjr7ei6shGHm8cjPSVFCYzlKgMHicvxubYJyU=\n",
"hashes": [
"fad7073c0b8b3d73dd61eadad6c1fe267b7caa6efef581af9d4d57728681e6b3",
"11a7c8eb98ad5ba52233abed26f320744082c4c8f68daf82d5f609264c067669",
"749c9ca1d2b12751d1ed5916e3e79f56bc67bffa28d7a1453421677b1e364485",
"76d989c0400a50765b1d8c86bc05a64374e2580dd4ccb9439b43c8d40056b6c3",
"c392d68963244c9e2b6ee8c1e86e4752b6bf2c0b75825302eabc2baec6c328f7",
"038dbf97ce93a2808c480ac7aa15bb727c9f1109ccd13b5ba75559db260e65a7",
"21a92612613df90299ba7ca6fbd47b9fd528b78518422763fb9049885ab89a05",
"8970a820614ded0ab40a27c891f3e3a1142304342972cfc534e7d121aa4fa3e1",
"87eb630e892d862a2a6b893c180006bf19de9ab7f1525991ae479c5844be3d10",
"1f762e7f648d56546b855e8d77b93071a3bba25b576f067b4bbf6164e43a67a0",
"08383fd08e0d3b80138d8e6ca4ab2d685ea79da1b805ce7a50f0ad3c59b0b30a",
"e3d9a82d92fb0df4d4cf59441893a35e4df510e37ae23eca86f2091926048fbb",
"3cda1a514a77dbdb460349a34ba4f755dcd85a755ae338e5033f6e5572eb6307",
"6c2db1d0574f019b34a46280cf3f38ce2a4937ea816da7efad192672fcde9b29",
"f782ddd94e6b75d4cede1132c7203e13ad793e5bff80630d244f642e29adccde",
"5d8b1ca6301b537730a86e4083ffb05534bba7c08ef38be053b7fef5f47d91ee",
"cd0d9f7bcf79e949fefbac0572994b304f73e626394164f14164abb550a6c9a3",
"74f801e4996a8332bfc30de5a49f1256da593c09a7f5b94f3677df835b6742a5",
"51e5d80682cc50abdb392ed3a0cb1aa1b946e1f4bff103d04d314620155e13bd",
"98c486feb5d87092a78a46c4b5be04868654900affc2e86ffb20074dc73a883a",
"6969c49bd73f19bf28a5eaeabd331ddd60502defb2cd3d96e17b741c80adec6c"
],
"logIndex": 63771522,
"rootHash": "1dfb0064d5d3ba352837a9564d94bdd9742a1769341ddadce986783a79dce642",
"treeSize": 63781918
},
"signedEntryTimestamp": "MEUCIFKfSMY/DU+jTzgnZCC52Id+5TYi87eCKjnjQF7EG7CXAiEA0CI+zIfoOGKrGtKhMSZ3rPb7VVPXrwk8n9paqsJFI3Y="
}
}
} Formatted {
"body": "eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiI3YTIxODlmNzNlYTVkYmVlMmQ1NjgxMGU4NjBjZDgxNjdlYTFiMGYzMTdkZGRjMmU0YzE2NmU3ZDY4NzUzOGYwIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FUUNJQlZtMVc1YlNSOXBTYVFyRWRVL2dOeEZuaVQzMCs4RGp5SG9naERwWHM3b0FpQXMyby82c3l2bzRaTDd3YVUrMXBNeVIvR1dNWlZTaUdzRm5hSm04ZDZZZ0E9PSIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCUVZVSk1TVU1nUzBWWkxTMHRMUzBLVFVacmQwVjNXVWhMYjFwSmVtb3dRMEZSV1VsTGIxcEplbW93UkVGUlkwUlJaMEZGUldGSWNHUmhZVE13ZEVOaVMxbG5lalpyUlhsQmNqTXJORmh5TWdwb1pWUjNaM2hQVVVrM2QwcDRiVWhIWW5OU1dYcEJWbFJyUVN0RGIzVjZUblZrTUc5eGRuUTRXVXB0Tm5CQlFUZG5SbUZFUkZGU05EUlJQVDBLTFMwdExTMUZUa1FnVUZWQ1RFbERJRXRGV1MwdExTMHRDZz09In19fX0=",
"integratedTime": 1706680021,
"logID": "c0d23d6ad406973f9559f3ba2d1ca01f84147d8ffc5b8445c224f98b9591801d",
"logIndex": 67934953,
"signedEntryTimestamp": "MEUCIFKfSMY/DU+jTzgnZCC52Id+5TYi87eCKjnjQF7EG7CXAiEA0CI+zIfoOGKrGtKhMSZ3rPb7VVPXrwk8n9paqsJFI3Y="
}
} Output of "SignedEntryTimestamp": "MEUCIBLPDfNvAm2Rw446aiM/E37l7iAWCBu58PrxZIv5TmLYAiEAufaHvGAB3zZPlykdT+HSkLPNiO62OUEIIby9/4E7MCY=",
"Payload": {
"body": "eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiJjOTA5YWVlYmQ5MzU3YzA4MGE3N2VhOWQzZWVmMzU1ODMwYTMxNTRjNTJmODlhOWJlMDUyMDMxN2ZlNTQ1NjdhIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FWUNJUUNrWHBBUUlpYlVpYkVaQ2FkSWptclBXbDdLMlpiTmdrdUtXU01WVkl6RkZRSWhBSkE5UHZJb2RGcythMmFWSGtBMFNDVlYvbVBPdG1jZlN0MTQ2ZU1lSjFiRyIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCUVZVSk1TVU1nUzBWWkxTMHRMUzBLVFVacmQwVjNXVWhMYjFwSmVtb3dRMEZSV1VsTGIxcEplbW93UkVGUlkwUlJaMEZGVmpkS2NERk9RbXh1TmxWWWFqSk5jVE5YTTJKRk1XRnRiM3BwVVFwUWJrbzBSVmg0TlVsaGExUndiMVp1WW1aWFZrazRZVXRNYTBJclQzTmxORUowZFRrNVVuVk5Ra2R1YjJVd1ZWRnZkelJPZEZwSUswTlJQVDBLTFMwdExTMUZUa1FnVUZWQ1RFbERJRXRGV1MwdExTMHRDZz09In19fX0=",
"integratedTime": 1706459446,
"logIndex": 67201951,
"logID": "c0d23d6ad406973f9559f3ba2d1ca01f84147d8ffc5b8445c224f98b9591801d"
}
} QUESTION: If Output of {
"base64Signature": "MEQCIBVm1W5bSR9pSaQrEdU/gNxFniT30+8DjyHoghDpXs7oAiAs2o/6syvo4ZL7waU+1pMyR/GWMZVSiGsFnaJm8d6YgA==",
"rekorBundle": {
"SignedEntryTimestamp": "MEUCIQDa3gvz9sKc3WCl1fMqdV6bT2MmeS7k3mPxAkN+UwuAgwIgLDdm0QELkfmMbdMpDCBnuASyPrdJuU/65Iiuq3T8EAU=",
"Payload": {
"body": "eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiI3YTIxODlmNzNlYTVkYmVlMmQ1NjgxMGU4NjBjZDgxNjdlYTFiMGYzMTdkZGRjMmU0YzE2NmU3ZDY4NzUzOGYwIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FUUNJQlZtMVc1YlNSOXBTYVFyRWRVL2dOeEZuaVQzMCs4RGp5SG9naERwWHM3b0FpQXMyby82c3l2bzRaTDd3YVUrMXBNeVIvR1dNWlZTaUdzRm5hSm04ZDZZZ0E9PSIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCUVZVSk1TVU1nUzBWWkxTMHRMUzBLVFVacmQwVjNXVWhMYjFwSmVtb3dRMEZSV1VsTGIxcEplbW93UkVGUlkwUlJaMEZGUldGSWNHUmhZVE13ZEVOaVMxbG5lalpyUlhsQmNqTXJORmh5TWdwb1pWUjNaM2hQVVVrM2QwcDRiVWhIWW5OU1dYcEJWbFJyUVN0RGIzVjZUblZrTUc5eGRuUTRXVXB0Tm5CQlFUZG5SbUZFUkZGU05EUlJQVDBLTFMwdExTMUZUa1FnVUZWQ1RFbERJRXRGV1MwdExTMHRDZz09In19fX0=",
"integratedTime": 1706680021,
"logIndex": 67934953,
"logID": "c0d23d6ad406973f9559f3ba2d1ca01f84147d8ffc5b8445c224f98b9591801d"
}
}
} |
Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> rename RekorBundle to RekorResponse Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> add e2e test for rekor bundle Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> added proper description Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> handle nil value for rekorBundle Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> fix e2e test failure Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> updated the logic Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> updated logic for rekor bundle Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com> specify whether bundle or rekor-bundle is passed Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com>
Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com>
Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com>
@haydentherapper any thought on this ? |
4838f9b
to
0c9fe65
Compare
Signed-off-by: Vivek Kumar Sahu <vivekkumarsahu650@gmail.com>
Hey sorry I will respond early next week! I have some ideas on how to simplify this |
Summary
This PR fixes the bugs while attaching the
rekor-bundle
into an image.Closes #3458
Release Note
Bug fixes and fixes of previous known issues
Documentation
OpenSSL
tool to generate key pair.openssl genrsa -out privkey.pem 4096
openssl rsa -in privkey.pem -outform PEM -pubout -out pubkey.pem
IMAGE_DIGEST=ghcr.io/viveksahu26/hi-cosign:main
cosign generate $IMAGE_DIGEST > payload.json
openssl dgst -sha256 -sign privkey.pem -out payload.json.sig payload.json
base64
cat payload.json.sig | base64 > payload.json.base64.sig
NEW_IMAGE=$(cosign triangulate ${IMAGE_DIGEST})
crane manifest ${NEW_IMAGE}
payload
,signature
, andpublic key
to rekor instance.rekor-cli upload --artifact payload.json --signature payload.json.sig --public-key pubkey.pem --pki-format x509
logIndex
oruuid
of the record.rekor-bundle
is added. Now check therekor-bundle
:rekor-cli get --log-index 60606559
rekor-bundle
to an Image, for that first you need to create arekor-bundle
by yourself.-
curl https://rekor.sigstore.dev/api/v1/log/entries\?logIndex\=60606559 | jq
NOTE: currently there is no such commands which automates the process of constructing a rekor bundle for us. Like
cosign generate
command create the payload on providing the image.- Now upload this
rekor-bundle
,signature
andpayload
to an Image.-
cosign attach signature --payload payload.json --signature payload.json.base64.sig --rekor-response rekor_bundle.json $IMAGE_DIGEST
NEW_IMAGE=$(cosign triangulate ${IMAGE_DIGEST})
crane manifest ${NEW_IMAGE}